Few improvements to qq compare
#937
Conversation
| # TODO: remove harcoded | ||
| # usually with heatmap first should be signal col = 1 | ||
| # then phase (col = 2) | ||
| # if there is another element it should be a fit done on the first column |
There was a problem hiding this comment.
Shall we raise a warning if j>2?
There was a problem hiding this comment.
Yes, we could. Actually here the comments are wrong because it should be (0) signal, (1) phase, (2) fit.
So yes, we could raise a warning.
There was a problem hiding this comment.
Actually I believe that given that we most likely are not going to be able to write a proper test for the j>2 we might skip the warning... what do you think @GabrielePalazzo?
There was a problem hiding this comment.
Ok. Let's leave the comment, just to remember it.
src/qibocal/cli/compare.py
Outdated
| for i in range(len(plots[0])): | ||
| fig0 = plots[0][i] | ||
| fig1 = plots[1][i] |
There was a problem hiding this comment.
Minor remark.
Can we zip plots[0] and plots[1]? It removes a variable and it's safer (even though we expect plots[0] and plots[1] to have the same length). We can either save the combined plot into a new list or we can reuse fig0.
| for i in range(len(plots[0])): | |
| fig0 = plots[0][i] | |
| fig1 = plots[1][i] | |
| for fig0, fig1 in zip(plots[0], plots[1]): |
There was a problem hiding this comment.
Implemented in 366b632
I've also changed from append_trace to add_trace since I believe that for what we are doing the result should be the same.
If it looks good to you feel free to merge @GabrielePalazzo
Some improvements to
qq compare:Checklist:
mastermainmain